-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FTheoryTools] Implement "ambient_space_models_of_g4_fluxes" #4268
[FTheoryTools] Implement "ambient_space_models_of_g4_fluxes" #4268
Conversation
39f340f
to
ae37641
Compare
A few more numbers for "the" big model @apturner @emikelsons :
For details on what I mean by "most likely", see the changes in |
ae37641
to
d0f4482
Compare
d0f4482
to
99a56d8
Compare
99a56d8
to
94f5ea4
Compare
Calling the new method on the SU(5)xU(1) restricted Tate model leads to an error, the following code reproduces the error: julia> Kbar = anticanonical_divisor_class(B3) julia> t = literature_model(arxiv_id = "1109.3454", julia> ambient_space_models_of_g4_fluxes(t, check = false); |
I think the problem lies on line 137 of the basis_of_h22 code, where an error occurs, if remaining_relations are empty |
Good catch. Thank you!
Indeed, I did not catch the case for which there are no remaining relations. 9e586c0 should fix this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
9e586c0
to
0f53f2d
Compare
0f53f2d
to
ed02e4e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4268 +/- ##
=======================================
Coverage 84.47% 84.47%
=======================================
Files 641 641
Lines 85394 85427 +33
=======================================
+ Hits 72133 72168 +35
+ Misses 13261 13259 -2
|
See the doc string for more details. Hopefully, this is sufficiently clear.
I have taken the liberty to profile this new function a lot, since we want to showcase this very soon (or so I believe). For the doctest example
g4_amb_list = ambient_space_models_of_g4_fluxes(qsm_model, check = false);
I found:The main improvement came from a more efficient identification of
remaining_vars_list
, but there were quite a number of other tricks I employed. Overall, I am very happy with these improvements.@apturner @emikelsons You may find it entertaining that for "the" big model, I expect that
ambient_space_models_of_g4_fluxes
completes in no more than 10 minutes on most personal computers. On mine, even only about 5 to 6 minutes. (The first run will of course always be somewhat slower...)